-
Notifications
You must be signed in to change notification settings - Fork 99
Introduce MessageViewModel
+ Show original translated message
#815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageContainerView.swift
Outdated
Show resolved
Hide resolved
SDK Size
|
Generated by 🚫 Danger |
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageContainerView.swift
Outdated
Show resolved
Hide resolved
StreamChatSwiftUITests/Tests/ChatChannel/MessageListViewAvatars_Tests.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageViewModel.swift
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageView.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a simpler way, without using environment object and involving the channel view model.
- we pass the message view model in the MessageContainerView, with default value. We can do that, it's not a breaking change.
- we introduce a state there for each message, whether the show original is tapped for that message.
- customers will implement the existing makeMessageContainer method, by passing their own VM to the regular view.
Let's discuss it when you are back, but I think we can simplify things here a bit.
.onAppear { | ||
displayedText = attributedString(for: message) | ||
} | ||
.onChange(of: message, perform: { updated in | ||
displayedText = attributedString(for: updated) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to see not having it here. To be honest, I am not sure why it was needed in the first place.
e3a800d
to
f890c13
Compare
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageListView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageViewModel.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageViewModel.swift
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageContainerView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageListConfig.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageViewModel.swift
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageViewModel.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/Reactions/ReactionsOverlayView.swift
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageContainerView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageContainerView.swift
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageViewModel.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
This reverts commit b3b00ef.
This reverts commit 43df718.
This reverts commit ab0e511. # Conflicts: # Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageContainerView.swift # StreamChatSwiftUITests/Tests/ChatChannel/MessageContainerView_Tests.swift
This reverts commit bd695bf. # Conflicts: # Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageListView.swift
This reverts commit ab8f758. # Conflicts: # StreamChatSwiftUITests/Tests/ChatChannel/MessageContainerView_Tests.swift
23198af
to
862593c
Compare
|
🔗 Issue Links
https://linear.app/stream/issue/IOS-794/show-original-translation
https://linear.app/stream/issue/IOS-823/customizing-message-view-display-logic
🎯 Goal
📝 Summary
Goal
The main goal of this PR is to introduce the new
MessageViewModel
which was required to properly implement the original translated message feature without breaking changes or too many hacks. Besides this, a lot of customers want to override the logic of the Message View, especially when to hide or show some views based on business logic.Requirement
One thing that was important to do was to have a way to share state from the
ChatChannelViewModel
to the newMessageViewModel
, since theMessageViewModel
should not have any internal state to not cause unnecessary re-renders. Besides that, since the message view is inside a LazyStack, the state would be overidden whenever the view is recreated. So it is important to provide the message view model to each message view whenever it is created.🛠 Implementation
Solution
The most common practice is to have item view models as the data of the
ListViewModel
. So instead ofmessages: [ChatMessage]
as the data, it would bemessages: [MessageViewModel]
. But because of ourLazyCachedMapCollection
, and to avoid breaking changes, we can't really do this.The final solution is to create a factory method in theChatChannelViewModel
, calledmakeMessageViewModel(message:)
. This approach allows us to pass data from the channel view model to the message view model, ensuring the view model is always updated whenever the view is re-created. The view model is then passed to theMessageContainerView
as an environment object. (Can't be @ObservedObject, otherwise it would be breaking)UPDATE: In order to avoid environment objects, the strategy was changed. The view model is now created in the
makeMessageContainerViewFactory()
, and the original translated messages state is controlled by a singleton. If a customer wants to show/hide the original text from the message actions, it is easy, here is an example: 33520f9🧪 Manual Testing Notes
☑️ Contributor Checklist
docs-content
repo